-
Notifications
You must be signed in to change notification settings - Fork 458
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
23971 Proposed API and schema changes #25013
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was going to ask whether we should revise the default on me?include_settings
but it looks like loading the hosts page pulls /me
anyway so we can just revise that call to pull column settings when we need them (vs. trying to economize on HTTP requests now).
3c1a8ad
to
ff956d3
Compare
Hey @rachaelshaw, hope you had a nice vacation. Assigning you this ticket, how do these specs look from product POV? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approach lgtm, left a few small comments
@rachaelshaw @lucasmrod @sgress454 added our decisions to this issue description, thanks for your insights! |
18c4e78
to
2615745
Compare
2615745
to
a1a3d1e
Compare
For #25034
API changes:
this PR diff ("available_teams" change is adding missing documentation for current API behavior)
schema changes:
users
table,settings
, typejson
. Defaults to{}
. New setting,hidden_host_columns
, added or updated on first relevant API call per user.semantics
"hidden_host_columns"
field means "not yet set, use defaults":{"settings":{"hidden_host_columns": null}}
"hidden_host_columns"
field means "no columns hidden, show all columns in the UI":{"settings":{"hidden_host_columns": []}}
Updates 1/7/25 per discussion with @rachaelshaw @lucasmrod @sgress454:
include_ui_settings=true
included withGET
s to/me
or/users/:id
will trigger considering the API call to be a contributor API call, giving more flexibility for future changes. Note that this is the first time we have one endpoint that can be conditionally considered a contributor endpoint depending on how it is called.